Skip to content

Fix ArgumentOutOfRangeException crash in Problems list - #355

Open
hendrikmennen wants to merge 1 commit into
mainfrom
fix/errorlist-datagrid-currency-crash
Open

Fix ArgumentOutOfRangeException crash in Problems list#355
hendrikmennen wants to merge 1 commit into
mainfrom
fix/errorlist-datagrid-currency-crash

Conversation

@hendrikmennen

Copy link
Copy Markdown
Contributor

Problem

The app crashes with:

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'index')
   at Avalonia.Collections.DataGridCollectionView.GetItemAt(Int32 index)
   at Avalonia.Collections.DataGridCollectionView.get_IsCurrentInSync()
   at Avalonia.Collections.DataGridCollectionView.AdjustCurrencyForRemove(Int32 index)
   at Avalonia.Collections.DataGridCollectionView.ProcessRemoveEvent(Object removedItem, Boolean isReplace)
   at Avalonia.Collections.DataGridCollectionView.ProcessCollectionChanged(NotifyCollectionChangedEventArgs args)
   at System.Collections.ObjectModel.ObservableCollection`1.OnCollectionChanged(NotifyCollectionChangedEventArgs e)
   at OneWare.AI.UI.ViewModels.AiProjectViewModel.RefreshDiagnostics(Boolean includeExpensiveChecks)

Root cause

Upstream bug in Avalonia's DataGridCollectionView: ProcessRemoveEvent does not check PassesFilter (unlike ProcessAddEvent).

When an item that is filtered out of the view is removed from the source collection:

  1. removeIndex = IndexOf(removedItem) is -1
  2. needToRemove is still true (internalRemoveIndex < (PageIndex + 1) * PageSize-1 < 0)
  3. AdjustCurrencyForRemove(-1) takes the index < CurrentPosition branch and does SetCurrent(CurrentItem, CurrentPosition - 1)CurrentPosition == -1 while CurrentItem is still in the view
  4. The CurrentPosition >= Count guard doesn't catch a negative position
  5. IsCurrentInSyncIsCurrentInView is trueGetItemAt(-1) throws

So it reproduces whenever a problem row at position 0 is current and a filtered-out error is removed — exactly what IErrorService.RefreshErrors does during an AI project diagnostics refresh.

Fix

Added BatchObservableCollection<T>, which suppresses change notifications while a batch of mutations is applied and raises a single Reset afterwards. DataGridCollectionView handles Reset via RefreshOrDefer()RefreshOverride(), which rebuilds the view and restores currency through ResetCurrencyValues, bypassing the broken incremental-remove path entirely.

All removal paths in ErrorListViewModel are now batched: RefreshErrors, ClearFile, Clear(source), Clear(project) and Clear(project, source). No Reset is raised if the batch didn't actually change anything, so unchanged diagnostics refreshes stay free.

Verification

Standalone repro against Avalonia.Controls.DataGrid:

before: cur=b pos=0
plain CRASH: ArgumentOutOfRangeException Specified argument was out of the range of valid values. (Parameter 'index')
batch: no crash, count=2 cur=b pos=0

The batched collection completes cleanly and preserves the current item/position. dotnet build src/OneWare.ErrorList/OneWare.ErrorList.csproj succeeds with 0 errors.

Avalonia's DataGridCollectionView.ProcessRemoveEvent does not check
PassesFilter (unlike ProcessAddEvent). When an item that is filtered out
of the view is removed from the source collection, removeIndex is -1, so
AdjustCurrencyForRemove(-1) takes the `index < CurrentPosition` branch and
sets CurrentPosition to -1 while CurrentItem is still in view. The
following IsCurrentInSync check then calls GetItemAt(-1) and throws.

In practice this crashed the app whenever a problem row at position 0 was
current and a filtered-out error was removed, e.g. from
IErrorService.RefreshErrors during AI project diagnostics refresh.

Route all removal paths of the error list through a new
BatchObservableCollection that suppresses notifications and raises a
single Reset, which DataGridCollectionView handles via a safe full
refresh that restores currency.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant